Conversation
…ommon/jwt/JwtTokenGenerator.java
jinkonu
left a comment
There was a problem hiding this comment.
많은 분들의 제출을 보진 못했지만
코드에서 굉장히 많은 고민과 노력을 하셨다는 걸 느낄 수 있었습니다!!
특히 어노테이션 활용하는 건 제가 배워가도록 하겠습니다 ㅎㅎ
| @@ -15,12 +18,15 @@ | |||
| @RequiredArgsConstructor | |||
| @EnableWebSecurity //web Security를 사용할 수 있게 | |||
| public class SecurityConfig { | |||
There was a problem hiding this comment.
개인적으로 config 파일은 따로 디렉토리를 생성해서 분류해주면 좋을 것 같네요!!
| public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { | ||
| return SecurityContextHolder.getContext() | ||
| .getAuthentication() | ||
| .getPrincipal(); | ||
| } |
There was a problem hiding this comment.
아마 여기서 매개변수를 파싱하는 것 같은데,
제가 이해했을 때에는 토큰 파싱보다는 서버에서 기억해둔 사용자 ID를 가져오는 듯한?
코드인 것 같은데 혹시 어떻게 되는 걸까요??
There was a problem hiding this comment.
여기서는 바인딩할 객체를 리턴해주는 코드입니다!!
그래서 여기서 getPrincipal로 userId를 가져옵니다!
| @Target(ElementType.PARAMETER) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface MemberId { | ||
| } |
| } catch(UnauthorizedException e){ | ||
| // throw new RuntimeException(String.valueOf(ErrorMessage.JWT_UNAUTHORIZED_EXCEPTION)); | ||
| } |
There was a problem hiding this comment.
제가 로그찍는거를 까먹었네요!
추가로 말씀드리자면!
여기서 throw를 던지면 ExceptionTranslationFilter 에 의해 동작하는 AuthenticationEntryPoint 을 상속한 CustomJwtAuthenticationEntryPoint 가 호출됩니다. 그러면 securityConfig에서 해준 permitAll이 먹지 않는데요! 이유는 아래 코드가 동작하지 않기 때문에, 필터가 넘어가지 않습니다. 그래서 여기서는 로그로만 찍었습니다.
filterChain.doFilter(request, response);자세한 내용은 아래 블로그 참고해주시면 감사하겠습니다!!
permitAll이 안돼서 고생했었는데, 아래 블로그에서 자세히 설명해주어서 새로 배웠습니다~
| } catch (EntityNotFoundException e) { | ||
| throw new NotFoundException(ErrorMessage.MEMBER_NOT_FOUND); | ||
| } |
There was a problem hiding this comment.
제가 원하는 메세지를 던져주려고 저렇게 했습니다!
다른 방법이 있을까요??
There was a problem hiding this comment.
getRefreshTokenFromRedis를 보면 이미 안에 NotFoundException을 던지는 것으로 보이는데 혹시 EntityNotFoundException가 터질 일이 있나요?? 어느 상황에 터지는걸까요?(진짜 단순 궁금)
junggyo1020
left a comment
There was a problem hiding this comment.
코드 안의 주석이 잘 작성되어 있어서 코드를 이해하기 쉬웠어요...!!! 에러 처리를 한 경우에 어떤 상황에서 예외처리를 진행하는지에 대한 주석도 추가되면 더 좋을 것 같아요 ㅎㅎ 정말 고생하셨습니다.
| public String generateToken(final Long userId, boolean isAccessToken) { | ||
| final Date presentDate = new Date(); | ||
| final Date expireDate = generateExpireDataByToken(isAccessToken, presentDate); |
There was a problem hiding this comment.
메서드명에 오타가 있는 것 같아요!! 'generateExpireDataByToken' -> 'generateExpireDateByToken'이 맞는 것 같습니다. 오타를 수정하면 더 메서드명이 직관적이고 좋을 것 같아요!!!
| //타입이 같은지 확인 | ||
| boolean isLongType = Long.class.isAssignableFrom(parameter.getParameterType()); | ||
|
|
There was a problem hiding this comment.
isAssignableFrom 메서드는 보통 서브클래스 관계를 확인하는데 사용되는데요!
'A.class.isAssinableFrom(B.class)'는 "B가 A의 서브클래스이거나 같은 클래스인지?"를 확인하는 반면 equals 메서드는 "두 클래스 객체가 정확히 같은지?" 확인하는 메서드입니다.
그래서 이 경우에 정확히 'Long' 클래스인지를 확인하는 것이기 때문에 parameter.getParameterType().equals(Long.class)를 사용하는 방법도 좋아보이네요..!!
참고 링크 남겨두겠습니다:)
docs.oracle.com/javase/8/docs/api/java/lang/Class.html#isAssignableFrom-java.lang.Class-
tkdwns414
left a comment
There was a problem hiding this comment.
안녕하세요 성준님. 처음으로 리뷰해보는데 진짜 정말정말 열심히 하시네요. 깜짝 놀랐습니다. 부족하지만 궁금한 점들과 함께 리뷰 남기고 갑니다!
| public void validateAccessToken(String accessToken) { | ||
| try { | ||
| parseToken(accessToken); | ||
| } catch (ExpiredJwtException e) { | ||
| throw new UnauthorizedException(ErrorMessage.EXPIRED_ACCESS_TOKEN); | ||
| } catch (MalformedJwtException ex) { | ||
| throw new UnauthorizedException(ErrorMessage.INVALID_ACCESS_TOKEN); | ||
| } catch (UnsupportedJwtException ex) { | ||
| throw new UnauthorizedException(ErrorMessage.UNSUPPORTED_ACCESS_TOKEN); | ||
| } catch (IllegalArgumentException ex) { | ||
| throw new UnauthorizedException(ErrorMessage.EMPTY_ACCESS_TOKEN); | ||
| } catch (SignatureException ex) { | ||
| throw new UnauthorizedException(ErrorMessage.JWT_SIGNATURE_EXCEPTION); | ||
| } | ||
| } | ||
|
|
||
| public void validateRefreshToken(String refreshToken) { | ||
| try { | ||
| parseToken(refreshToken); | ||
| } catch (ExpiredJwtException e) { | ||
| throw new UnauthorizedException(ErrorMessage.EXPIRED_REFRESH_TOKEN); | ||
| } catch (Exception e) { | ||
| throw new UnauthorizedException(ErrorMessage.INVALID_REFRESH_TOKEN_VALUE); | ||
| } | ||
| } |
There was a problem hiding this comment.
atk와 rtk의 내용이 다르지 않다면 같은 validate를 쓰는 것도 나쁘지 않을 것 같은데 따로 분리한 이유가 있을까요?
There was a problem hiding this comment.
에러 메세지가 다르기 때문에 나눠주었습니다!!
| .body( | ||
| userJoinResponse | ||
| ); | ||
| @PostMapping("signup") |
There was a problem hiding this comment.
헉 감사합니다!!
그리고 아래 getRefreshTokenFromRedis쪽에 남겨주신거에 제 커맨트를 남길수가 없어서 여기다가 대신 남깁니다!
저도 지금 다시 보니 현재 제 생각도 상준님말씀처럼 EntityNotFoundException가 터질일이 없을 것 같네요!
감사합니다!!
| } catch (EntityNotFoundException e) { | ||
| throw new NotFoundException(ErrorMessage.MEMBER_NOT_FOUND); | ||
| } |
There was a problem hiding this comment.
getRefreshTokenFromRedis를 보면 이미 안에 NotFoundException을 던지는 것으로 보이는데 혹시 EntityNotFoundException가 터질 일이 있나요?? 어느 상황에 터지는걸까요?(진짜 단순 궁금)
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항
1. Jwt
역할에 따라 JwtGenerator, JwtProvider, JwtValidator로 나누었습니다.
2. RTR(Refresh Token Rotation)
만약 refresh token을 탈취당한다면 refresh token이 만료되기 전까지 이 refresh token으로 계속 access token을 발급해낼 수 있기 때문에 RTR 방식으로 코드를 구현했습니다.
그래서 멤버 가입을 할 때 뿐만 아니라, accessToken 재발급을 할 때도 accessToken과 refreshToken을 재발급해줍니다.
3. UserIdArgumentResolver
accessToken을 가지고 매 컨트롤러에서 해줘야 하는 token파싱대신 Argument Resolver를 이용하면 중복 로직을 제거할 수 있습니다.